New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix a false positive for Style/StringLiterals
#10166
Fix a false positive for Style/StringLiterals
#10166
Conversation
…correctly. Two cases are improved here: 1. Escaped double quotes (`\"`) within a double-quoted string are handled correctly (replaced with an unescaped double quote) when converting to a single-quoted string. 2. Escaped backslashes (`\\`) are treated properly and not doubled for both single- and double-quoted strings.
Makes sense. I take the intention here is to limit this to recognised control character sequences and quote escaping? Because it's worth noting even things like |
Hey folks, I believe there are more examples of false-positives apart from the |
@@ -17,7 +17,7 @@ def wrong_quotes?(src_or_node) | |||
# 1. It contains a double quote | |||
# 2. It contains text that would become an escape sequence with double quotes | |||
# 3. It contains text that would become an interpolation with double quotes | |||
!/" | (?<!\\)\\[abcefMnrtuUx0-7] | \#[@{$]/x.match?(src) | |||
!/" | (?<!\\)\\[abcefMnrstuUx0-7] | \#[@{$]/x.match?(src) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\k
, \A
, \z
, \Z
are also missing
86b37c0
to
d06047a
Compare
Follow up to rubocop@a7ef72a#r57503462. This PR fixes a false positive for `Style/StringLiterals` when using some meta characters (e.g. `'\s'`, `'\z'`) with `EnforcedStyle: double_quotes`.
d06047a
to
0039834
Compare
To clarify my point here: this applies to any character. Or at least I can't find any that behave differently. Am I missing something?
|
With this change, |
Yeah, I'm seeing a lot of similar problems across our codebase. I'm not entirely convinced about the rationale behind this change - we use double quoted strings everywhere, unless the string needs literal backslashes in it (where we use single-quotes to avoid the double-backslash).
Correct - in all your examples, none of the double-quoted strings have a backslash in their 'output', and all the single-quoted ones do. |
Follow up to rubocop#10166 (comment). This PR fixes a false positive for `Style/StringLiterals` when `EnforcedStyle: double_quotes` and using single quoted string with backslash. It has moved a separate implementation for `Style/QuotedSymbols` to resolve rubocop#10152 to `Style/QuotedSymbols` to prevent impact on `Style/StringLiterals`'s false positives.
Yup. I've opened #10229 to prevent the false positives. |
Follow up to a7ef72a#r57503462.
This PR fixes a false positive for
Style/StringLiterals
when using'\s'
withEnforcedStyle: double_quotes
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.